-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ws: Conditionally block channel requests to remote hosts #21003
ws: Conditionally block channel requests to remote hosts #21003
Conversation
From a quick look this looks correct, but we probably want to have a test for this? Do we also show a proper error then if a user navigates to an old bookmark? The only question I have about the code is, what is filled in |
Good point, I didn't consider checksums urls... I'll check. (In practice, checksums are only known the the shell once a manifest has been loaded from a host, but this is of course about closing all the doors and not rely on what the shell is doing.) |
Yeah, via curl.
They get redirected by the shell. This change here should have no effect with our current shell. It should never construct URLs for remote hosts when AllowMultiHost is false. But the shell is a lot of code, and there might be bugs, so we should also block the requests we don't want to happen in cockpit-ws. |
They are handled by |
eed5f67
to
75c332c
Compare
Done. |
75c332c
to
0c64a1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
0c64a1e
to
fc9a506
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! If you have to push again, please remove the redundancy, but not that important.
testUrlRoot is unhappy 😢 |
When AllowMultiHost is false, cockpit-ws will reject all GET requests that would load from a non-localhost bridge.
fc9a506
to
b02575a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cheers!
When AllowMultiHost is false, cockpit-ws will reject all GET requests that would load from a non-localhost bridge.